5.0.0 #122
Merged
5.0.0 #122
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…re checking file extension for .txt output
Add structured text output for AI model context
Use an alternate method of reading column details if the old way fails
There was a problem hiding this comment.
Pull Request Overview
This release adds support for text-based ER diagram outputs alongside images, implements a structured Markdown representation for .txt files, and updates testing, documentation, and dependency requirements to align with these features.
- Introduce
--text-outputflag and handle.txtextension for structured text output in the CLI command - Implement
generateStructuredTextRepresentationinGraphBuilderand extend schema column retrieval - Add new PHPUnit tests for both raw DOT text and structured Markdown outputs, update README, CI matrix, and bump PHP/DBAL versions
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/GenerationTest.php | Added tests for raw text output, .txt structured output, and snapshot test |
| src/GraphBuilder.php | New generateStructuredTextRepresentation method and enhanced column fetching logic |
| src/GenerateDiagramCommand.php | Added --text-output option, .txt extension handling, and helper for text filenames |
| composer.json | Bumped PHP requirement to ^8.2 and tightened Doctrine DBAL versions |
| README.md | Documented new text and structured Markdown output options |
| .github/workflows/tests.yml | Expanded CI matrix to include PHP 8.2–8.4 and Laravel 10–12 |
Comments suppressed due to low confidence (2)
composer.json:19
- Bumping PHP support to ^8.2 removes compatibility with earlier 8.x and 7.x versions. This is a breaking change; please update your changelog or release notes to inform consumers.
"php": "^8.2",
src/GraphBuilder.php:116
- Laravel's Schema facade does not provide a
getColumnsmethod. Consider usingSchema::getColumnListingand then retrieving type info via the Doctrine schema manager or another supported API.
return Schema::getColumns($model->getTable());
Comment on lines
+132
to
+138
| if (is_object($column)) { | ||
| $name = $column->getName(); | ||
| $typeName = $column->getType()->getName(); | ||
| } else { // it's an array! | ||
| $name = $column['name'] ?? ''; | ||
| $typeName = $column['type_name'] ?? ''; | ||
| } |
There was a problem hiding this comment.
[nitpick] Handling two different column representations (object vs array) here adds complexity. It might be cleaner to normalize the column data in getTableColumnsFromModel so downstream code only deals with a single, consistent structure.
Suggested change
| if (is_object($column)) { | |
| $name = $column->getName(); | |
| $typeName = $column->getType()->getName(); | |
| } else { // it's an array! | |
| $name = $column['name'] ?? ''; | |
| $typeName = $column['type_name'] ?? ''; | |
| } | |
| $name = $column['name']; | |
| $typeName = $column['type_name']; |
Copilot uses AI. Check for mistakes.
Comment on lines
+75
to
+100
| // First check for text-output option | ||
| if ($this->option('text-output') || $this->option('format') === self::FORMAT_TEXT) { | ||
| $textOutput = $graph->__toString(); | ||
|
|
||
| // If text-output option is set, write to file | ||
| if ($this->option('text-output')) { | ||
| $outputFileName = $this->getTextOutputFileName(); | ||
| file_put_contents($outputFileName, $textOutput); | ||
| $this->info(PHP_EOL); | ||
| $this->info('Wrote text diagram to ' . $outputFileName); | ||
| return; | ||
| } | ||
|
|
||
| // Otherwise just output to console | ||
| $this->info($textOutput); | ||
| return; | ||
| } | ||
|
|
||
| $graph->export($this->option('format'), $this->getOutputFileName()); | ||
| // Then check for .txt extension in filename | ||
| $outputFileName = $this->getOutputFileName(); | ||
| if (pathinfo($outputFileName, PATHINFO_EXTENSION) === 'txt') { | ||
| // Generate structured text output for .txt files | ||
| $textOutput = $this->graphBuilder->generateStructuredTextRepresentation($models); | ||
| file_put_contents($outputFileName, $textOutput); | ||
| $this->info(PHP_EOL); | ||
| $this->info('Wrote structured ER diagram to ' . $outputFileName); |
There was a problem hiding this comment.
[nitpick] The command handler has multiple return points and duplicated file-write logic for text and structured outputs. Consider refactoring into smaller methods (e.g., handleTextOutput & handleStructuredOutput) to improve readability and reduce duplication.
Suggested change
| // First check for text-output option | |
| if ($this->option('text-output') || $this->option('format') === self::FORMAT_TEXT) { | |
| $textOutput = $graph->__toString(); | |
| // If text-output option is set, write to file | |
| if ($this->option('text-output')) { | |
| $outputFileName = $this->getTextOutputFileName(); | |
| file_put_contents($outputFileName, $textOutput); | |
| $this->info(PHP_EOL); | |
| $this->info('Wrote text diagram to ' . $outputFileName); | |
| return; | |
| } | |
| // Otherwise just output to console | |
| $this->info($textOutput); | |
| return; | |
| } | |
| $graph->export($this->option('format'), $this->getOutputFileName()); | |
| // Then check for .txt extension in filename | |
| $outputFileName = $this->getOutputFileName(); | |
| if (pathinfo($outputFileName, PATHINFO_EXTENSION) === 'txt') { | |
| // Generate structured text output for .txt files | |
| $textOutput = $this->graphBuilder->generateStructuredTextRepresentation($models); | |
| file_put_contents($outputFileName, $textOutput); | |
| $this->info(PHP_EOL); | |
| $this->info('Wrote structured ER diagram to ' . $outputFileName); | |
| // Handle text output if the option is set or format is 'text' | |
| if ($this->option('text-output') || $this->option('format') === self::FORMAT_TEXT) { | |
| $this->handleTextOutput($graph); | |
| return; | |
| } | |
| // Handle structured output for .txt files | |
| $outputFileName = $this->getOutputFileName(); | |
| if (pathinfo($outputFileName, PATHINFO_EXTENSION) === 'txt') { | |
| $this->handleStructuredOutput($models, $outputFileName); |
Copilot uses AI. Check for mistakes.
…s more robustly, accommodating both object and array formats for column data.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.